[Feature/satellite] styling visibility - other updates#877
[Feature/satellite] styling visibility - other updates#877MichaelWheeley wants to merge 15 commits intoaccius:Stagingfrom
Conversation
pass tle data from useSatellites.js
# Conflicts: # src/hooks/useSatellites.js
accius
left a comment
accius
left a comment
There was a problem hiding this comment.
Nice feature — station altitude and min elevation are solid additions for sat tracking. A few things to address before merge:
Must fix
Broken HTML in satellite card — The 3 inner <table> elements are nested directly inside the outer <table> without proper <tr><td> wrapping. This is invalid HTML and will render inconsistently across browsers. Either close the outer table before starting the sections, or wrap each inner table in <tr><td colspan="2">.
Duplicate style attribute on position table:
<table style="background-color: #302115; width:100%; font-size:11px; style="color: #888; padding:2px 0;"">Second style= is ignored — the color: #888 never applies.
valueAsNumber with ?? — e.target.valueAsNumber returns NaN when empty, not null/undefined, so ?? 100 won't catch it. NaN will propagate to the saved config. Use || 100 or an explicit isNaN check instead.
Hardcoded background colors (#302115, #233b46, #00f800, #252e17) — these will look wrong in Light and Retro themes. Should use CSS variables or at least rgba() with low opacity over the existing background.
Should fix
minElev default mismatch — SettingsPanel defaults to 5.0 but useSatelliteLayer.js uses config?.satellite?.minElev || 0. A user who hasn't opened settings will see different visibility behavior between the layer and the hook.
Negative min elevation allowed — The input allows -89 which would mark a satellite as "visible" when below the horizon. Should probably clamp to 0–89.
Nit
Squash the merge commits on merge — 13 commits (4 are merge syncs) is noisy for the history.
color changes replace ?? with || incase of NaN minElev default consistency to 5.0 restrict minElev to -5 to 89
Handled by removal of inner tables with additional styling on elements.
fixed duplicate style property
replaced ?? with || incase of NaN
removed all # hardcoded colors, replaced with rgba(), modify opacity on selected colors
minElev consistency default to 5.0
Restricted minElev to -5 to 89 allowing for marginal below the horizon setting. |
|
2 participants
What does this PR do?
reference #855
(miscellaneous tle1, tle2 passed - will use them in next iteration)
Type of change
How to test
Checklist
server.js: caches have TTLs and size caps (we serve 2,000+ concurrent users)var(--accent-cyan), etc.).bak,.old,console.logdebug lines, or test scripts includedScreenshots (if visual change)
station altitude and minimum elevation boxes
not visible
satellite above 0deg but below minimum threshold to be considered visible (I have it set to 5.1deg)
satellite visible, elevation shown as 5deg however internally it is greater than or equal to 5.1deg. Tried it but displaying decimal degrees here doesn't look good. I could have made the minimum elevation setting an integer but was happy with this arrangement.
